Backup Annotations #116
Backup Annotations #116
Conversation
…d to backup pods. Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
| InitContainers: pool.InitContainers, | ||
| SidecarContainers: pool.SidecarContainers, | ||
| ExtraEnv: pool.ExtraEnv, | ||
| Annotations: vts.Spec.Annotations, |
There was a problem hiding this comment.
The field vts.Spec.Annotations is for annotations on the VitessShard object. I think what we want is pool.Annotations?
Also, did you check that this field is respected for vtbackup Pods even though it wasn't being filled in before?
There was a problem hiding this comment.
Done. Also, I'm not sure what you mean by respected. Would you mind re-phrasing that question?
There was a problem hiding this comment.
The code that builds vtbackup Pods is separate from the code that builds vttablet Pods. Only some of these vttablet spec fields are relevant to vtbackup Pods, so by default a field added here will have no effect on vtbackup Pods unless we go fix the vtbackup Pod code to look at the new field.
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
There was a problem hiding this comment.
This looks good. I'm also unsure what @enisoc meant by the field being respected for vtbackup pods, but other than that it's solid.
| @@ -306,7 +306,7 @@ func vttabletSpecs(vts *planetscalev2.VitessShard, parentLabels map[string]strin | |||
| drain.SupportedAnnotation: "ensure that the tablet is not a master", | |||
| } | |||
| update.Annotations(&annotations, pool.Annotations) | |||
|
|
|||
| update.Annotations(&annotations, backupLocation.Annotations) | |||
There was a problem hiding this comment.
I think we need to watch out for the case when backupLocation is nil. Unlike vtbackup, we still reconcile vttablets when backups are not configured.
This PR adds support to pass annotations down to both backup pods and the backup sub-controller. A use case for this feature is to support IAM access.